Skip to content

HADOOP-18687. hadoop-auth: remove unnecessary dependency on json-smart#5524

Merged
steveloughran merged 1 commit intoapache:trunkfrom
web-iq:HADOOP-18687
Apr 6, 2023
Merged

HADOOP-18687. hadoop-auth: remove unnecessary dependency on json-smart#5524
steveloughran merged 1 commit intoapache:trunkfrom
web-iq:HADOOP-18687

Conversation

@mjwiq
Copy link
Contributor

@mjwiq mjwiq commented Mar 31, 2023 •
edited
Loading

Description of PR

https://issues.apache.org/jira/browse/HADOOP-18687

json-smart is not used by hadoop-auth and the reason for including it (for nimbus-jose-jwt) is no longer valid since that package has json-smart shaded now.

How was this patch tested?

I ran the tests for hadoop-auth

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
  • Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

@mjwiq
Copy link
Contributor Author

mjwiq commented Mar 31, 2023 •
edited
Loading

I did not remove the entry from pom.xml in hadoop-project because it also sets the version for json-smart that is included as a transitive dependency in hdfs:

[INFO] org.apache.hadoop:hadoop-hdfs-client:jar:3.4.0-SNAPSHOT
[INFO] \- org.mock-server:mockserver-netty:jar:5.11.1:test
[INFO]    \- org.mock-server:mockserver-core:jar:5.11.1:test
[INFO]       \- com.jayway.jsonpath:json-path:jar:2.4.0:test
[INFO]          \- net.minidev:json-smart:jar:2.4.7:test

@hadoop-yetus
Copy link

hadoop-yetus commented Mar 31, 2023

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 37s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+0 🆗 xmllint 0m 0s xmllint was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ trunk Compile Tests _
+0 🆗 mvndep 15m 58s Maven dependency ordering for branch
+1 💚 mvninstall 26m 15s trunk passed
+1 💚 compile 23m 3s trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚 compile 20m 30s trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚 mvnsite 1m 26s trunk passed
+1 💚 javadoc 1m 25s trunk passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚 javadoc 1m 18s trunk passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚 shadedclient 97m 19s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 29s Maven dependency ordering for patch
+1 💚 mvninstall 0m 36s the patch passed
+1 💚 compile 22m 36s the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚 javac 22m 36s the patch passed
+1 💚 compile 20m 29s the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
-1 ❌ javac 20m 29s /results-compile-javac-root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09.txt root-jdkPrivateBuild-1.8.0_362-8u362-ga-0ubuntu120.04.1-b09 with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu120.04.1-b09 generated 1 new + 2625 unchanged - 0 fixed = 2626 total (was 2625)
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 mvnsite 1m 24s the patch passed
+1 💚 javadoc 1m 19s the patch passed with JDK Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1
+1 💚 javadoc 1m 19s the patch passed with JDK Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
+1 💚 shadedclient 27m 20s patch has no errors when building and testing our client artifacts.
_ Other Tests _
+1 💚 unit 0m 31s hadoop-project in the patch passed.
+1 💚 unit 3m 21s hadoop-auth in the patch passed.
+1 💚 asflicense 0m 56s The patch does not generate ASF License warnings.
176m 46s
Subsystem Report/Notes
Docker ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5524/1/artifact/out/Dockerfile
GITHUB PR #5524
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint
uname Linux 5ed0a8fd4f03 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision trunk / 5a6bb63
Default Java Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
Multi-JDK versions /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.18+10-post-Ubuntu-0ubuntu120.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_362-8u362-ga-0ubuntu1~20.04.1-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5524/1/testReport/
Max. process+thread count 555 (vs. ulimit of 5500)
modules C: hadoop-project hadoop-common-project/hadoop-auth U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5524/1/console
versions git=2.25.1 maven=3.6.3
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@steveloughran steveloughran changed the title HADOOP-18687 Remove unnecessary dependency on json-smart HADOOP-18687. hadoop-auth: remove unnecessary dependency on json-smart Apr 6, 2023
steveloughran
steveloughran approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
LGTM. any hdfs test dependency shouldn't affect anything in production, and the way maven works, probably won't go downstream

@steveloughran steveloughran merged commit e45451f into apache:trunk Apr 6, 2023
@steveloughran
Copy link
Contributor

steveloughran commented Apr 6, 2023

well spotted -thanks!

merged to trunk. can you make a PR for branch-3.3 and submit it through yetus as well, to see how it goes. thanks

@steveloughran
Copy link
Contributor

steveloughran commented Apr 6, 2023

ooh, had a thought here. I wonder if it makes it into the binary distro now, it if doesn't, we should review those LICENSE files and cut the reference.

@pjfanning
Copy link
Member

pjfanning commented Apr 7, 2023

json-smart has a CVE - would it be possible to upgrade com.jayway.jsonpath/json-path to the latest version (which also uses the latest version of json-smart - that is CVE free)?

@steveloughran
Copy link
Contributor

steveloughran commented Apr 11, 2023

so what artifacts do we need to pull in to be free of json-smart pain?

@pjfanning
Copy link
Member

pjfanning commented Apr 11, 2023

json-path lib needs json-smart but if we upgrade the json-path lib, we could theoretically remove the explicit json-smart dependency. I could do a PR as a POC.

rohit-kb pushed a commit to rohit-kb/hadoop that referenced this pull request May 5, 2023
@mjwiq @rohit-kb
HADOOP-18687. hadoop-auth: remove unnecessary dependency on json-smart (
9cf91da 
apache#5524)


Contributed by Michiel de Jong
steveloughran pushed a commit that referenced this pull request May 9, 2023
@rohit-kb
HADOOP-18687. Remove json-smart dependency. (#5549 + #5524)
771c89a 
Contains 

* HADOOP-18687. hadoop-auth: remove unnecessary dependency on json-smart (#5524)
 Contributed by Michiel de Jong
* HADOOP-18687. Remove json-smart dependency. (#5549).
  Contributed by PJ Fanning.
ferdelyi pushed a commit to ferdelyi/hadoop that referenced this pull request May 26, 2023
@mjwiq @ferdelyi
HADOOP-18687. hadoop-auth: remove unnecessary dependency on json-smart (
df7299d 
apache#5524)


Contributed by Michiel de Jong
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@steveloughran steveloughran steveloughran approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mjwiq @hadoop-yetus @steveloughran @pjfanning